Preserve Claude usage source after login#1971
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 11, 2026, 8:48 PM ET / July 12, 2026, 00:48 UTC. Summary Reproducibility: yes. at source level with high confidence: current main unconditionally sets OAuth after login, and the canonical issue records OAuth failure followed by recovery when Auto is restored. This read-only review did not complete a live OAuth login. Review metrics: 4 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Successful Claude login should enable the provider while retaining the user’s selected source, with the controller regression and provider documentation preserving that contract. Do we have a high-confidence way to reproduce the issue? Yes, at source level with high confidence: current main unconditionally sets OAuth after login, and the canonical issue records OAuth failure followed by recovery when Auto is restored. This read-only review did not complete a live OAuth login. Is this the best way to solve the issue? Yes. Removing the overwrite is the narrowest maintainable solution because it honors persisted user settings and relies on the existing Auto fallback planner rather than creating a parallel policy. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 18ad6953060f. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (11 earlier review cycles; latest 8 shown)
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Maintainer decision requested. This PR is now scoped to the narrow fix ClawSweeper recommended:
The remaining ClawSweeper blocker is not another code issue. It wants either:
I am not running live Claude/OAuth/Keychain proof unprompted because |
|
Updated the PR body with clean-worktree after-fix proof at Proof added:
@clawsweeper re-review |
|
Added stronger behavior proof in This no longer stops at the policy helper:
Verified: No external fork code was downloaded or used. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Live proof attempt added to the PR body. What I could verify without exposing secrets:
Live OAuth completion is blocked externally:
So the proof state is now:
@steipete could you accept this as the proof override ClawSweeper asks for, or provide/run a maintainer-side redacted live OAuth transcript? @clawsweeper re-review |
|
Current maintainer action needed after the latest ClawSweeper rerun:
The current branch head is |
|
Pre-tag maintainer ask / final gate status:
I attempted to approve the fork CI run from this account, but GitHub returned @steipete please approve the fork CI run and, if you agree, accept the current proof override so this can be merged before tagging. |
c98779b to
d2a6563
Compare
|
Maintainer repair and exact-head proof for d2a6563:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Fixes #1970.
Summary
oauth.Current-main verification
steipete/CodexBarmainon 2026-07-07; latest checked SHA:aa401f1d8b7412b6a71abb6d13f57f5efc64d2ad.Sources/CodexBar/Providers/Claude/ClaudeLoginFlow.swiftstill forces.oauthafter successful login on currentmain.SettingsStoreTestsstill expects the default Claude source to be.auto.git merge-treeinto refreshedupstream/mainhas no conflict and leaves the PR scoped to the Claude login source policy, focused test, and Claude docs.Why
CodexBar defaults Claude to Auto, and Auto is the fallback-capable source mode. The login flow currently persists
oauthafter the runner reports success. If OAuth credentials are not readable afterward, that removes the fallback path and can leave the app repeatedly asking for Claude credentials.Preserving the selected source keeps the default setup resilient: default Auto remains Auto, so OAuth can still be preferred when available, but CLI/Web fallback remains available when OAuth is not usable. Explicit source choices remain explicit.
After-fix proof
Verified on 2026-07-07 at PR head
c98779b7b2b03bc95da8fac409492d8a8faa559e(origin/codex/claude-login-auto-source).What the stronger proof covers:
runClaudeLoginFlow()still callsClaudeLoginRunner.run(timeout:onPhaseChange:)unchanged.StatusItemController.runClaudeLoginFlowpost-login code path instead of stopping at the policy helper.SettingsStore,UsageStore, andStatusItemControllerinstances, simulates Claude login phases plus a successful runner result, then asserts:true,loginPhasereturns to.idle,settings.setProviderEnabled, andClaudeUsageDataSourcecase (auto,api,oauth,web,cli) is preserved after successful login.Claude loginwithoutcome=successfor each source, followed byProvider mode updated provider=claude value=auto/api/oauth/web/cli, proving the patched success path leaves the intended source intact.Relevant source anchors:
Sources/CodexBar/Providers/Claude/ClaudeLoginFlow.swiftTests/CodexBarTests/ClaudeLoginFlowPolicyTests.swiftCommand output:
Commands run
swift test --filter ClaudeLoginFlowPolicyTestsswift test --filter ClaudeSourcePlannerTestsswift test --filter ClaudeOAuthTestsmake checkmake test(exits on the existing unrelatedAdaptiveRefreshTimerTests.swift:117failure noted below)Existing unrelated test failures observed
make testfails inAdaptiveRefreshTimerTests.swift:117, testmanual mode performs the initial refresh but no recurring ticks: expected completed refresh count1, got2.swift test --no-parallel --filter AdaptiveRefreshTimerTestsreproduces the same failure outside this PR.swift test --filter SettingsStoreTestsalso fails inSettingsStoreTests.swift:1236, testmenu observation token ignores merged switcher selection churn: expecteddidChange.get()to be false but it was true.swift test --no-parallel --filter "menu observation token ignores merged switcher selection churn"reproduces the same failure.Live-provider proof
Attempted on 2026-07-07 at PR head
c98779b7b2b03bc95da8fac409492d8a8faa559eafter explicit live/browser proof escalation.What could be verified live without exposing credentials:
2.1.201 (Claude Code).claude auth statusreports a realclaude.ailogin withloggedIn: true,apiProvider: firstParty, andsubscriptionType: max(email/org redacted from PR text).Live OAuth login attempt result:
claude auth login --claudeailaunched a real OAuth flow and waited for a code.agent-browserreached Claude/CloudflarePerforming security verificationand could not advance past human verification.Sign in - Claude; completing it would require interactive credentials/human browser verification. The waiting CLI was stopped; no token, cookie, authorization code, or Keychain payload was captured or posted.What this proves:
source: claude).autoinstead of forcingoauth.Remaining proof gap:
Screenshots
Not applicable; this is a provider settings/source policy and docs change with no UI changes.